-
Notifications
You must be signed in to change notification settings - Fork 410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add StoreAndInstantiateContract gov proposal #1074
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1074 +/- ##
==========================================
+ Coverage 59.27% 60.33% +1.06%
==========================================
Files 53 54 +1
Lines 6758 7211 +453
==========================================
+ Hits 4006 4351 +345
- Misses 2456 2549 +93
- Partials 296 311 +15
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice start! It looks very complete. I added some minor notes.
From a framework design perspective, we should not expose the CreateAndInstantiate
method anywhere. This opens another path into the system. Let's stick with create and instantiate are 2 separate steps in keepers.
x/wasm/types/exported_keepers.go
Outdated
@@ -51,6 +51,16 @@ type ContractOpsKeeper interface { | |||
fixMsg bool, | |||
) (sdk.AccAddress, []byte, error) | |||
|
|||
CreateAndInstantiate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to export this.
x/wasm/types/proposal.go
Outdated
msg RawContractMessage, | ||
funds sdk.Coins, | ||
) *StoreAndInstantiateContractProposal { | ||
return &StoreAndInstantiateContractProposal{title, description, runAs, wasmBz, permission, unpinCode, admin, label, msg, funds} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use keys to init objects. Golang can generate the constructor for you.
x/wasm/keeper/contract_keeper.go
Outdated
@@ -128,3 +140,15 @@ func (p PermissionedKeeper) SetContractInfoExtension(ctx sdk.Context, contract s | |||
func (p PermissionedKeeper) SetAccessConfig(ctx sdk.Context, codeID uint64, caller sdk.AccAddress, newConfig types.AccessConfig) error { | |||
return p.nested.setAccessConfig(ctx, codeID, caller, newConfig, p.authZPolicy) | |||
} | |||
|
|||
func (p PermissionedKeeper) CreateAndInstantiate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove. No need to expose this
x/wasm/keeper/contract_keeper.go
Outdated
@@ -23,6 +23,18 @@ type decoratedKeeper interface { | |||
authZ AuthorizationPolicy, | |||
) (sdk.AccAddress, []byte, error) | |||
|
|||
createAndInstantiate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to optimize for this worflow. Let's stick with the existing create and instantiate methods when used as a library
|
||
// StoreAndInstantiateContractProposal gov proposal content type to store | ||
// and instantiate the contract. | ||
message StoreAndInstantiateContractProposal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 This looks complete
} | ||
|
||
// ValidateBasic validates the proposal | ||
func (p StoreAndInstantiateContractProposal) ValidateBasic() error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a test for regression
x/wasm/types/proposal.go
Outdated
Label: %s | ||
Msg: %q | ||
Funds: %s | ||
`, p.Title, p.Description, p.RunAs, p.WASMByteCode, p.Admin, p.Label, p.Msg, p.Funds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InstantiatePermission and unpinCode are missing.
Admin: p.Admin, | ||
Label: p.Label, | ||
Msg: string(p.Msg), | ||
Funds: p.Funds, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unpinCode is missing
@@ -226,6 +226,41 @@ func InstantiateContractProposalFixture(mutators ...func(p *InstantiateContractP | |||
return p | |||
} | |||
|
|||
func StoreAndInstantiateContractProposalFixture(mutators ...func(p *StoreAndInstantiateContractProposal)) *StoreAndInstantiateContractProposal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 good fixture
Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
7fb18b2
to
f4ac252
Compare
* Add Developer's guide and best practices * Fix comments
* preserve contract created date on genesis import and add query contract created date * add validate created * fix sims test app import export * add preserve contract history * Make proto-all only * Remove ResetFromGenesis * Add validation Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/spf13/viper/releases) - [Commits](spf13/viper@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/spf13/viper dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This reverts commit cf81eb8.
…om/prometheus/client_golang-1.14.0 Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0
Return contract history updates on query
* Add contract authz proto * Implement contract autorization * Register contract authz * Add contract-authz tests * Consume gas for contract authz * Add contract authz cli * Update cli usage * Model spike * Add max funds limit * Redesign authz model * Start e2e test * Full e2e test * Test filter and limits * Test accept * Fix description * No linter warning Co-authored-by: Giancarlos Salas <me@giansalex.dev>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Almost done :-)
After rebasing and fixing the 2 issues in ProposalStoreAndInstantiateContractCmd
I was able to submit a proposal via CLI:
wasmd tx gov submit-proposal store-instantiate "x/wasm/keeper/testdata/reflect.wasm" \
'{}' --label "alex" \
--title "testing" --description "Testing" --deposit "100ustake" \
--run-as $(wasmd keys show -a validator) \
--admin $(wasmd keys show -a validator) \
--amount 123ustake \
--from validator --gas auto --gas-adjustment=1.5 -y --chain-id=testing --node=http://localhost:26657 -b block -o json
and query it via wasmd q gov proposals --node=http://localhost:26657
.
@@ -525,3 +525,12 @@ func toStdTxResponse(cliCtx client.Context, w http.ResponseWriter, data wasmProp | |||
} | |||
tx.WriteGeneratedTxResponse(cliCtx, w, baseReq, msg) | |||
} | |||
|
|||
func EmptyRestHandler(cliCtx client.Context) govrest.ProposalRESTHandler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -23,7 +23,7 @@ func ProposalStoreCodeCmd() *cobra.Command { | |||
Short: "Submit a wasm binary proposal", | |||
Args: cobra.ExactArgs(1), | |||
RunE: func(cmd *cobra.Command, args []string) error { | |||
clientCtx, err := client.GetClientTxContext(cmd) | |||
clientCtx, proposalTitle, proposalDescr, deposit, err := getProposalInfo(cmd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good refactoring. There was lot's of DRY
x/wasm/client/cli/gov_tx.go
Outdated
Args: cobra.ExactArgs(3), | ||
Use: "store-instantiate [wasm file] [json_encoded_init_args] --label [text] --title [text] --description [text] --run-as [address] --admin [address,optional] --amount [coins,optional]", | ||
Short: "Submit and instantiate a wasm contract proposal", | ||
Args: cobra.ExactArgs(1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to be 2 or fails
Args: cobra.ExactArgs(1), | |
Args: cobra.ExactArgs(2), |
cmd.Flags().String(flagRunAs, "", "The address that is stored as code creator. It is the creator of the contract and passed to the contract as sender on proposal execution") | ||
cmd.Flags().String(flagInstantiateByEverybody, "", "Everybody can instantiate a contract from the code, optional") | ||
cmd.Flags().String(flagInstantiateNobody, "", "Nobody except the governance process can instantiate a contract from the code, optional") | ||
cmd.Flags().String(flagInstantiateByAddress, "", "Only this address can instantiate a contract instance from the code, optional") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also required to define now:
cmd.Flags().StringSlice(flagInstantiateByAnyOfAddress, []string{}, "Any of the addresses can instantiate a contract from the code, optional")
* Add StoreAndInstantiateContract gov proposal * Add integration tests * Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0 Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Remove exposed functions * Add tests * Add Developer's guide and best practices (CosmWasm#1075) * Add Developer's guide and best practices * Fix comments * Change genesis preserving contract history (CosmWasm#1076) * preserve contract created date on genesis import and add query contract created date * add validate created * fix sims test app import export * add preserve contract history * Make proto-all only * Remove ResetFromGenesis * Add validation Co-authored-by: Alex Peters <alpe@users.noreply.github.com> * Return contract history updates * Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082) Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/spf13/viper/releases) - [Commits](spf13/viper@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/spf13/viper dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)" This reverts commit cf81eb8. * Add cli and refactor code * Contract authz - redesign (CosmWasm#1077) * Add contract authz proto * Implement contract autorization * Register contract authz * Add contract-authz tests * Consume gas for contract authz * Add contract authz cli * Update cli usage * Model spike * Add max funds limit * Redesign authz model * Start e2e test * Full e2e test * Test filter and limits * Test accept * Fix description * No linter warning Co-authored-by: Giancarlos Salas <me@giansalex.dev> * Add StoreAndInstantiateContract gov proposal * Add integration tests * Remove exposed functions * Add tests * Add cli and refactor code * Fix comments Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com> Co-authored-by: Alex Peters <alpe@users.noreply.github.com> Co-authored-by: Giancarlos Salas <me@giansalex.dev>
* Add StoreAndInstantiateContract gov proposal * Add integration tests * Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0 Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Remove exposed functions * Add tests * Add Developer's guide and best practices (CosmWasm#1075) * Add Developer's guide and best practices * Fix comments * Change genesis preserving contract history (CosmWasm#1076) * preserve contract created date on genesis import and add query contract created date * add validate created * fix sims test app import export * add preserve contract history * Make proto-all only * Remove ResetFromGenesis * Add validation Co-authored-by: Alex Peters <alpe@users.noreply.github.com> * Return contract history updates * Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082) Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/spf13/viper/releases) - [Commits](spf13/viper@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/spf13/viper dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)" This reverts commit cf81eb8. * Add cli and refactor code * Contract authz - redesign (CosmWasm#1077) * Add contract authz proto * Implement contract autorization * Register contract authz * Add contract-authz tests * Consume gas for contract authz * Add contract authz cli * Update cli usage * Model spike * Add max funds limit * Redesign authz model * Start e2e test * Full e2e test * Test filter and limits * Test accept * Fix description * No linter warning Co-authored-by: Giancarlos Salas <me@giansalex.dev> * Add StoreAndInstantiateContract gov proposal * Add integration tests * Remove exposed functions * Add tests * Add cli and refactor code * Fix comments Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com> Co-authored-by: Alex Peters <alpe@users.noreply.github.com> Co-authored-by: Giancarlos Salas <me@giansalex.dev>
* Add StoreAndInstantiateContract gov proposal * Add integration tests * Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0 Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Remove exposed functions * Add tests * Add Developer's guide and best practices (CosmWasm#1075) * Add Developer's guide and best practices * Fix comments * Change genesis preserving contract history (CosmWasm#1076) * preserve contract created date on genesis import and add query contract created date * add validate created * fix sims test app import export * add preserve contract history * Make proto-all only * Remove ResetFromGenesis * Add validation Co-authored-by: Alex Peters <alpe@users.noreply.github.com> * Return contract history updates * Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082) Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0. - [Release notes](https://github.com/spf13/viper/releases) - [Commits](spf13/viper@v1.13.0...v1.14.0) --- updated-dependencies: - dependency-name: github.com/spf13/viper dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)" This reverts commit cf81eb8. * Add cli and refactor code * Contract authz - redesign (CosmWasm#1077) * Add contract authz proto * Implement contract autorization * Register contract authz * Add contract-authz tests * Consume gas for contract authz * Add contract authz cli * Update cli usage * Model spike * Add max funds limit * Redesign authz model * Start e2e test * Full e2e test * Test filter and limits * Test accept * Fix description * No linter warning Co-authored-by: Giancarlos Salas <me@giansalex.dev> * Add StoreAndInstantiateContract gov proposal * Add integration tests * Remove exposed functions * Add tests * Add cli and refactor code * Fix comments Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com> Co-authored-by: Alex Peters <alpe@users.noreply.github.com> Co-authored-by: Giancarlos Salas <me@giansalex.dev>
Closes #785